-
Notifications
You must be signed in to change notification settings - Fork 167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update fit2obs for COM refactor #1719
Update fit2obs for COM refactor #1719
Conversation
@jack-woollen I get the following error in the logs, though I think it is unrelated to the path change:
Full log: https://gist.github.com/WalterKolczynski-NOAA/d7be426a446c3444d8364c6591dda04c Not sure if this expected or indicates another bug. Working directory (Orion):
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WalterKolczynski-NOAA I'm home after being in the hospital for a couple days. I can't look into this until next week. Please rerun your test with the current tag. Thanks.
@WalterKolczynski-NOAA Okay, taking another look at the logs while more awake, the problem is if you want to test the code and avoid missing file messages you need to provide 126 hour forecasts in the vrfyarch for 5 days prior to the verification date. |
Thanks @jack-woollen, I'll run it out longer and report back. Well wishes for your recovery! This can wait until you are better. |
@jack-woollen Ran out to six days and still the same files missing.
Analysis ones populate fine, just the forecast ones aren't created. |
@WalterKolczynski-NOAA For some reason still not getting the prior forecast days in the mix. Below is a list of outputs from the orion fitx directory, followed by outputs from the operational version for the other day. files created from the test run: jwoollen@Orion-login-2:/work2/noaa/stmp/wkolczyn/RUNDIRS/fit2obs> ls -ltr fit2obs.235040/fitx f00.acar.2021122600 adpsfc.anl.2021122600 files created in production (different date): ./fits ./horiz/anl: ./horiz/fcs: |
I found the previous problem (it was unrelated to fit2obs directly). I've updated the output log. Here is the file inventory now:
I don't see any obvious errors in the log. Is this complete? @jack-woollen |
@WalterKolczynski-NOAA Looks better but i don't don't see any f120 files in the fits directory and there is no horiz directories. To get f120 files you need a 126 hour forecasts since it does time interpolation, as well as 5 prior days. |
Sorry, missed horiz directory with that wildcard:
Not sure why it cuts off at |
Looks like I just had to run the forecast longer:
|
601fb20
to
cef278b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @WalterKolczynski-NOAA, now looks good.
Automated global-workflow Testing Results:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
See a question in the review below.
jobs/JGDAS_FIT2OBS
Outdated
@@ -17,11 +17,15 @@ export CDATE | |||
vday=${CDATE:0:8} | |||
vcyc=${CDATE:8:2} | |||
|
|||
export COM_INA=${ROTDIR}/gdas.${vday}/${vcyc}/atmos | |||
YMD=${vday} HH=${vcyc} generate_com -rx COM_ATMOS_ANALYSIS_VERIF:COM_ATMOS_ANALYSIS_TMPL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is COM_ATMOS_ANALYSIS_VERIF
used in the fit2obs scripts? Or is it being used to set COM_INA
on line 24?
Same with COM_OBS_VERIF
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to set COM_INA
and COM_PRP
below. We can cut out the middle man if you want, but I believe I followed this same pattern in other j-jobs that had to set variables for external packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to set
COM_INA
andCOM_PRP
below. We can cut out the middle man if you want, but I believe I followed this same pattern in other j-jobs that had to set variables for external packages.
Then we should atleast eliminate -x
.
I would rather directly set COM_INA
and COM_PRP
and cut out the middle man. There is no reason to call 2 things with the same name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Fit2obs had been turned off during the COM refactor because it had not yet been updated for the new paths. Now that the job has been updated, it can be turned back on. Refs: NOAA-EMC#1486
4baaa62
to
50ab97b
Compare
Automated global-workflow Testing Results:
|
Automated global-workflow Testing Results:
|
@TerrenceMcGuinness-NOAA these logs are still not clearing properly when the process restarts. |
It might be a simple fix in that the |
This was an anomaly because I forced the restart on Hera without re-building on Hera today because it was only the case files in the PR that were wrong. Notice the Orion's started fresh. I messed up the restart there and Walter added new case files to the PR and resubmitted it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good.
Automated global-workflow Testing Results:
|
Automated global-workflow Testing Results:
|
Automated global-workflow Testing Results:
|
Automated global-workflow Testing Results:
|
Automated global-workflow Testing Results:
|
Automated global-workflow Testing Results:
|
Automated global-workflow Testing Results:
|
Description
Updates the fit2obs j-job for COM refactor. Also corrects a related error in the archive job where the wrong source was being used for history files to be copied to the verification archive.
Fixes #1486
Type of change
How Has This Been Tested?
Checklist